Remove unnecessary tabindex attributes from offcanvas docs examples#41524
Remove unnecessary tabindex attributes from offcanvas docs examples#41524anotherjames wants to merge 1 commit into
Conversation
In an early commit of the original Offcanvas component's PR (twbs#29017), the tabindex attribute on the wrapping div was manually adjusted in javascript. By the time of that PR's final state, that no longer happened, but the initial tabindex attribute remained in the code examples in its docs. Since Safari can do unexpected things with div[tabindex] elements, it is worth removing the now-unnecessary attribute from the examples to avoid obscure bugs.
|
Thanks for reporting this issue and submitting a PR, @anotherjames! Thinking out loud: since the offcanvas component behaves similarly to a modal in terms of focus management, if we move forward with this change, we might want to consider whether a similar adjustment should also apply to modals—assuming it doesn't negatively impact accessibility or focus behavior. |
|
unfortunately, simply removing the Video: using Chrome/NVDA, looking at the deploy preview. triggering one of the example offcanvases, we see focus still remains on the trigger button in the underlying page. using cursor up/down to read/navigate in NVDA, we're still going through the page behind the offcanvas. it's only if we hit TAB once the offcanvas is open that we end up inside the offcanvas and "trapped". for comparison, video includes the current behaviour, where focus is programmatically moved to the offcanvas. bootstrap-offcanvas.mp4Now, moving focus to the offcanvas itself (as with modals) can indeed have unintended side effects - for instance, you may not want its entire contents to be read out in one big go. A better solution is to programmatically move focus to some element at the top/start of the offcanvas... |
|
Interesting! Why is a |
|
|
Huh, OK, I see! Well maybe there's an idea in here somewhere, but probably needs something with wider thought then. I just hope we all keep testing our sites in Safari whilst it continues to behave 'differently' due to the tabindex. So easy to miss! :) |
3 participants
Description
Adjusts code examples in the offcanvas component's documentation in order to avoid potential bugs in Safari due to its unusual focus handling.
Motivation & Context
Having copied the code examples of the offcanvas component in an implementation for a website, I was surprised to find the presence of the
tabindexattribute on the wrapping<div>caused unexpected behaviour in Safari. I had an event listener for thefocusinevent on an<a>element within the offcanvas component. When clicking that link element, Safari sets the event target to the<div>, unlike Chrome which sets it to the<a>. (Safari has a history of unusual focus handling!) This meant my event listener acted on the wrong element, causing a bug report from my client.In an early commit of the original Offcanvas component's PR (#29017), the
tabindexattribute on the wrapping<div>was being manually adjusted in javascript. But that no longer happens (e.g. since #33865 changed the way keyboard navigation & focus was handled anyway), yet the initialtabindexattribute remained in the code examples in its docs. Given that the wrapping<div>element wouldn't be tabbable without thetabindexattribute anyway, I believe the attribute is unnecessary. I suggest we remove it to save other developers who copy the examples from similar headaches!Type of changes
Checklist
npm run lint)Live previews
Related issues